-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create Helm chart GitHub action #224
base: main
Are you sure you want to change the base?
Create Helm chart GitHub action #224
Conversation
Thanks for this PR! I’m at a conference through tomorrow. I will take a look at this PR on Monday. Sorry for the delay. |
Thanks for the additions. I think we need the helm chart first before merging this PR. Per https://github.com/helm/chart-releaser-action I like and have previously used the convention of Additionally, I feel this needs to be a separate job from |
@DavidNix Thanks for the review, I will work to have that deployed into |
I have created the charts in the directory /charts, however they still need testing before adding the directory to the releaser action config file. Is there a Strangelove dev environment that can be used? I can deploy to my microk8s cluster at home, but I don't want to be solving platform bugs that wouldn't happen in gcp, as making it agnostic can happen in a further PR. |
There is but we have competing priorities at the moment, so we won't be able to set up such an environment for a few sprints. I think microk8s or similar may be the best bet for now. I'll endeavor to look over this PR by EOW. Thank you! Fwiw, for the future, I'm hoping there's a pattern where we can use the output of |
Yes I can make that a part of this PR. I did it by hand but can add a pipeline step that passes the output of kustomize to helmify, then we should be able to use those charts as the basis for the release action. Will have that done today, but no rush on your end, I understand things may take priority over QOL enhancements, but hope to make development easier for you and the team :) |
Pushing up the latest changes, we currently remove the old |
Apologies for my belated response. Last week was unusually busy. A couple issues:
After some thought, my suggestion is to remove the CICD in this PR and work on just the helm chart itself. We can manually test the helm chart generation + deployment in a test cluster. We add back the CICD workflow in a later PR. |
We may not be able to 100% make a helm chart from kustomize build given the user will want to customize attributes in a |
on: | ||
pull_request: | ||
types: | ||
- closed | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This workflow file looks great. But we actually only want to release on tags. Also, as I mentioned in a comment, I recommend pulling the work into a separate PR.
# incremented each time you make changes to the application. Versions are not expected to | ||
# follow Semantic Versioning. They should reflect the version the application is using. | ||
# It is recommended to use it with quotes. | ||
appVersion: "0.1.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, apiVersion
matches the latest git tag which corresponds to the latest operator release.
I think I agree with everything you stated, and appreciate your input. The PR route was my preferred method as well, but didn't want to change until I had your approval. Will look to break these out these out tonight as I have time. Definitely a 2 part PR. I have tested the helm deployment, tho not with everything configured, and can confidently say it deploys, however I think some further testing is in order. :D |
Thank you, I appreciate your understanding here and your effort! FWIW, at Strangelove, we use the kustomize present in the |
Apologies on the delay, this has not been abandoned I've just been sick recently, should wrap this up today & tomorrow. |
@justyntemme @DavidNix Is there an update on this PR by chance? I was looking into the Helm chart route for deploying the operator. I'd be happy to help test out if needed. Thanks! |
This adds the GitHub action to automatically create a helm chart on push to main. I will be testing the generated helm chart locally, and adding the custom configuration it may need for any block storage, open ports, or custom dns information among other changes to the default helm chart.